feat(account-keychain): implement TIP-1053 nonces#3838
Open
legion2002 wants to merge 15 commits intomainfrom
Open
feat(account-keychain): implement TIP-1053 nonces#3838legion2002 wants to merge 15 commits intomainfrom
legion2002 wants to merge 15 commits intomainfrom
Conversation
Contributor
|
legion2002
force-pushed
the
feat/tip-1053-key-authorization-nonce
branch
from
45873c0 to
a37532b
Compare
legion2002
force-pushed
the
feat/tip-1053-key-authorization-nonce
branch
from
1e29a90 to
34a663c
Compare
legion2002
requested review from
0xKitsune,
0xrusowsky,
fgimenez,
klkvr,
mattsse and
rakita
as code owners
tempoxyz-cyclops-bot
left a comment
tempoxyz-cyclops-bot
left a comment
There was a problem hiding this comment.
👁️ Cyclops Review
PR #3838 refactors TIP-1053 nonces from a zero-sentinel B256 into an explicit Option<B256>. Encoding determinism (trailing(canonical)), pre-T5 gating across validate_env / precompile / dispatch, atomic per-account (account, nonce) consumption, storage-layout shift, and intrinsic-gas accounting all check out. One verified actionable finding remains around txpool visibility of nonce consumption (inline). See workers' Reviewer Callouts below.
Reviewer Callouts
- ⚡
burnKeyAuthorizationNonceis the first stateful AccountKeychain mutation callable by access keys.ensure_nonce_burn_calleronly requirestx_origin == msg_senderplus an active access key; per-call scope rules are enforced one layer up. Because the calldata first word is the nonce (not an address),recipientsscoping cannot meaningfully restrict which nonces a scoped key can burn. Confirm this broadening for legacy/unscoped keys is intentional before T5. - ⚡ Same-tx auth+use ordering. Any future change that lets an access key drive
burnKeyAuthorizationNoncebefore the symmetricensure_admin_callergate runs must keepensure_nonce_burn_callerin sync. - ⚡ Reverted batch permanently burns the nonce. Per design, KeyAuthorization state commits before the AA batch executes; if the batch reverts, the key remains authorized and the TIP-1053 nonce remains consumed. Worth surfacing in wallet/UX docs.
- ⚡
is_legacy_compatibleis currently unreferenced. Updated to considerhas_nonce()but has no callers. Confirm whether a downstream encoder/serializer is expected to consult it before T5. - ⚡ Provider/SDK gap.
crates/alloy/src/provider/keychain.rslacks helpers forauthorizeKey_2andburnKeyAuthorizationNonce; SDK consumers must hand-encode until helpers are added.
legion2002
commented
May 7, 2026
Contributor
📊 Tempo Precompiles CoverageprecompilesCoverage: 5665/7842 lines (72.24%) File details
contractsCoverage: 1/258 lines (0.39%) File details
Total: 5666/8100 lines (69.95%) |
tempoxyz-cyclops-bot
left a comment
tempoxyz-cyclops-bot
left a comment
There was a problem hiding this comment.
👁️ Cyclops Review — PR #3838 (TIP-1053 Key Authorization Nonces)
Summary
PR #3838 implements TIP-1053: an optional bytes32 nonce field in KeyAuthorization payloads with per-account single-use enforcement, gated behind T5. Three workers traced the full lifecycle (RLP encoding → signature hash → handler validation → precompile dispatch → storage write → event extraction → pool eviction) and core invariants hold (legacy byte-equivalence on nonce: None, signature hash binds the nonce, T5 hardfork gating in dispatch + handler + precompile, pool/execution parity, single-use storage, atomicity of pre-checks before nonce consumption). One verified gas-undercharge bug remains — see inline comment.
Reviewer Callouts
- ⚡ Pre-consume validation fragility (
crates/precompiles/src/account_keychain/mod.rs:287-302): The "fail before consuming the nonce" property holds only as long asapply_key_authorization_restrictions/replace_allowed_callskeep re-validating the same conditions thatt3_spending_limit_capandvalidate_call_scopescover. Adding a new check inapply_*not mirrored in pre-validate would let a bad payload burn a nonce while rolling back the auth. - ⚡ Open keychain checkpoint pattern (
crates/revm/src/handler.rs:1267-1420, pre-existing): When the keychainStorageCtx::enterclosure returnsErr,?propagates andkeychain_checkpointis left open; nonce-burn atomicity now also relies on the upstream block executor reverting state for invalid txs. Not introduced by this PR, but the new persistent write expands the blast radius. - ⚡ Access-key burn power (
burnKeyAuthorizationNonce):ensure_nonce_burn_callerlets any active access key burn nonces, gated only by the access key's call-scope rules (crates/precompiles/src/account_keychain/mod.rs:1057-1069). Wallets that grant unrestricted access keys are implicitly granting "burn any nonce". Spec-consistent, but a real footgun for SDK/wallet UX. - ⚡
is_bad_transaction()classification of nonce reuse (crates/revm/src/error.rs:298):KeychainPrecompileErroris treated as state-dependent, butKeyAuthorizationNonceAlreadyUsedis permanent — the tx will never become valid again. Misclassification keeps peers re-broadcasting these txs. Minor DoS surface; one-line allowlist for the specific subcase would help. - ⚡ Reorg consistency for nonce eviction (
crates/transaction-pool/src/maintain.rs:398-420): When a block emittingKeyAuthorizationNonceConsumedis reorged out, sibling AA txs evicted on that event are not automatically re-admitted — same class as SIGP-TMPO2-22. The nonce-eviction set should be invalidated/recomputed on reorg alongsiderevoked_keys/spending_limit_*. - ⚡
is_legacy_compatible()is now dead code (crates/primitives/src/transaction/key_authorization.rs:312-313): Updated to disqualify nonce-bearing auths but no caller exists. Confirm intent before any future caller is added. - ⚡ T5 activation checklist: All workers note this feature is dormant on the active T2 hardfork. Before T5 is scheduled, re-review the live execution paths with T5 enabled — especially direct AccountKeychain caller behavior, pool hygiene under real nonce-consumption events, and the post-
consume_key_authorization_noncefailure paths incrates/precompiles/src/account_keychain/mod.rs:317-342, 1072-1080. - ⚡ Gas pinning regression: The new pinned test at
crates/revm/src/handler.rs:3081-3090only asserts the delta between base and nonce variants; anyBUFFERvalue satisfies it. Add an absolute-value pinning test against an independently computed expected gas including all events emitted on the path.
…horization-nonce # Conflicts: # tips/verify/lib/tempo-std
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements TIP-1053 key authorization nonces behind T5 using the simplified nonce-presence semantics from the spec PR. Present nonce fields are included in the signed key authorization and consumed per account on successful authorization; absent nonce fields preserve legacy byte-equivalent encodings. Adds T5-gated AccountKeychain selectors to authorize with a nonce, burn a nonce, and query nonce consumption.